Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HDF5 1.12.3 #487

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add HDF5 1.12.3 #487

wants to merge 7 commits into from

Conversation

dipterix
Copy link
Contributor

No description provided.

@dipterix
Copy link
Contributor Author

Hi I would like to get hdf5r compiled on webr repo. Is there any way I can test if this PR has all the components needed to install that package?

  • The HDF5 version is 1.12.3 because this is a version that both hdf5r and rhdf5 support.
  • In the hdf5/targets.mk, I don't know values to assign to OPTIONAL_HDF5_BINS. I put h5cc because it's the executable program filename.

@dipterix dipterix marked this pull request as ready for review September 23, 2024 04:06
Copy link
Member

@georgestagg georgestagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for taking the time to write this PR!

It looks like the hdf5 sources are not being built yet, because the make variables need to be named OPTIONAL_WASM_LIBS and OPTIONAL_WASM_BINS to be included in the make all command. Unfortunately, that means the tests have passed but not actually tested your changes.

Once I made that change, some issues have arisen. You should be able to see this yourself by renaming the make variables as suggested, then running:

cd libs; make all

I have made a first pass at correcting the issues, and included the required changes to get building up and running in this review.

I find that the build currently fails with the following:

LD_LIBRARY_PATH="$LD_LIBRARY_PATH`echo -L/Users/georgestagg/work/webr/wasm/lib -s USE_BZIP2=1 -s USE_ZLIB=1 -Oz -fwasm-exceptions -s SUPPORT_LONGJMP=wasm |                  \
                sed -e 's/-L/:/g' -e 's/ //g'`"                               \
         ./H5make_libsettings  H5lib_settings.c  ||                               \
            (test $HDF5_Make_Ignore && echo "*** Error ignored") ||          \
            (rm -f H5lib_settings.c ; exit 1)
/bin/sh: ./H5make_libsettings: Permission denied
make[2]: *** [H5lib_settings.c] Error 1
make[1]: *** [install-recursive] Error 1
emmake: error: 'make install' failed (returned 2)
make: *** [/Users/georgestagg/work/webr/wasm/lib/libhdf5.a] Error 1

Once the changes below have been applied, I will come back later and take another pass at getting hdf5 to compile for Wasm. Possibly H5make_libsettings is an intermediate binary and we just need to patch configure to run it through node, or something similar.

Once the resulting library is compiling and available, it should be that the R package "just works", but we will see.

libs/recipes/hdf5/targets.mk Outdated Show resolved Hide resolved
libs/recipes/hdf5/rules.mk Outdated Show resolved Hide resolved
libs/recipes/hdf5/rules.mk Outdated Show resolved Hide resolved
@georgestagg
Copy link
Member

georgestagg commented Oct 1, 2024

Is there any way we can use a newer version of HDF5? I have tested and am able to build v1.14.5, because there have been come changes in the HDF5 source that much improve the process of cross-compilation.

@dipterix
Copy link
Contributor Author

dipterix commented Oct 2, 2024

Please let me check if package hdf5r and rhdf5 work with the new versions. The latest header version for hdf5r is 1.12.0 and for rhdf5 is 1.10.0. I'm not sure whether newer versions will work.

Also when compiling the package, I encountered this error:

wasm-ld: error: unable to find library -lsz

Maybe I should make a PR to add libaec as well...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants